fix(fetch): sanitize header tuples to fix Node.js 24 compatibility#766
fix(fetch): sanitize header tuples to fix Node.js 24 compatibility#766KrantiPonala wants to merge 1 commit intomswjs:mainfrom
Conversation
Node.js 24 internally passes a third boolean argument to Headers.prototype.set() and .append(). This extra argument was being recorded in raw header tuples, causing "Headers constructor: expected name/value pair to be length 2, found 3" errors when those tuples were later passed to the Headers constructor. - Only record [name, value] in set/append proxies, ignoring extra args - Sanitize header tuples when cloning Headers instances Fixes mswjs#762
kettanaito
changed the title
| // Use only the first two arguments (name, value) to record raw headers. | ||
| // Node.js 24+ may pass additional internal arguments that should not | ||
| // be included in the raw headers array. | ||
| recordRawHeader(thisArg, [args[0], args[1]], 'append') |
There was a problem hiding this comment.
Do you have a reference to Node.js implementation of those additional arguments to Headers? What do they affect?
By not recording them, are we risking recreating a Headers instance that's not a 1-1 representation of the one recorded?
There was a problem hiding this comment.
The 3rd boolean argument is passed internally during Headers operations — it's not part of the public Headers API spec which only defines set(name, value) and append(name, value) with 2 arguments.
Regarding 1-1 representation: No risk here. The boolean is purely internal (likely for validation skipping or provenance tracking). It's never exposed through any public Headers API (entries(), get(), forEach(), etc.), so it cannot affect actual header name/value data. The recorded raw headers will still be a 1-1 representation of the actual header data.
| // Use only the first two arguments (name, value) to record raw headers. | ||
| // Node.js 24+ may pass additional internal arguments that should not | ||
| // be included in the raw headers array. | ||
| recordRawHeader(thisArg, [args[0], args[1]], 'append') |
There was a problem hiding this comment.
What are your thoughts on doing args.slice(0, 2)? I suppose we can benchmark which would be faster here.
There was a problem hiding this comment.
Happy to change it to args.slice(0, 2) if you prefer — it's arguably more readable and explicitly conveys "take first 2 elements."
Performance-wise, [args[0], args[1]] is marginally faster (direct property access vs method call), but the difference is negligible in this context.
Let me know your preference and I'll update the PR.
|
Could you review the changes @kettanaito. This fix would help us move forward with node24 upgrade in our repo. thanks! |
|
hey @kettanaito , were you able to review this PR. Would need help in merging this. Kindly review at the earliest. So we could use node24 in our repo. This is a blocker for us to move forward. |
|
Looking for some movement on this @kettanaito I looked into the "third parameter" behavior that this PR addresses. In Node.js 24+, the public
The extra value we were seeing is not part of the Fetch API. It comes from Undici's internal header handling ( request.headersList.delete("proxy-authorization", true);That second argument ( This behavior implements the most likely the Fetch spec's authentication entries rules: https://fetch.spec.whatwg.org/#authentication-entries. Related Undici advisory: GHSA-3787-6prv-h9w3 Because the Fetch standard only defines headers as |
|
@kettanaito Bumping this so it doesn't get closed out |
3 participants
Node.js 24 internally passes a third boolean argument to Headers.prototype.set() and .append(). This extra argument was being recorded in raw header tuples, causing "Headers constructor: expected name/value pair to be length 2, found 3" errors when those tuples were later passed to the Headers constructor.
Fixes #762